Skip to content

Add .downscale(), .upscale(), and .rescale() methods #141

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

VeckoTheGecko
Copy link
Contributor

@VeckoTheGecko VeckoTheGecko commented May 14, 2025

Just done downscale for now. Logging off for tonight. Will work on upscale tomorrow (and incorporate #18 (comment) - thanks for the info about hierarchical tooling @keewis , was just looking for that)

Will add tests tmrw as well

@VeckoTheGecko VeckoTheGecko changed the title Add .downscale() .upscale() and .rescale()` methods Add .downscale(), .upscale(), and .rescale() methods May 14, 2025
if offset == 0:
return obj

upper_cell_membership = np.floor(obj.cell_ids / (4**offset))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, not sure how expensive this line is with high offsets?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there should be a constant cost (a binary right-shift), so I wouldn't worry too much about it. This implementation doesn't do that, so I'd recommend switching to healpix_geo.nested.zoom_to (nothing for ring, as hierarchy operations don't really make sense there)

@keewis
Copy link
Collaborator

keewis commented May 19, 2025

I forgot to mention this before, but usually we create new methods on the grid_info objects (in particular, the DGGSInfo base class) and then call that from the accessor. If a implementation doesn't support this operation (or if it just was not implemented yet), that's where you'd raise the NotImplementedError. In this case, though, h3ronpy has a change_resolution function that gives us what we need.

I'd also either return the groupby object, as that would increase the flexibility of the method (if / when xarray supports chaining operations like groupby + weighted or coarsen + weighted we'd then get that for free)

@VeckoTheGecko
Copy link
Contributor Author

I'd also either return the groupby object, as that would increase the flexibility of the method (if / when xarray supports chaining operations like groupby + weighted or coarsen + weighted we'd then get that for free)

Agreed, I think that sounds good now that we aren't considering rescale (#18 (comment)).

I'm still interested in this issue, but I have quite a bit on my plate with work and can't dedicate much to hobbyist coding. Might only get around to this again end June. Feel free to take over if this is blocking - otherwise I can pick this back up down the line.

@VeckoTheGecko
Copy link
Contributor Author

Might only get around to this again end June

Scratch that - mainly focussing on work, Rust learning, and other learning so i don't think I have time to get around to this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants